refactor(action-bar): use core tokens #2639
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tachometer resultsChromeaccordion permalink
action-bar permalink
action-button permalink
action-group permalink
action-menu permalink
button-group permalink
button permalink
card permalink
checkbox permalink
dialog permalink
field-group permalink
field-label permalink
help-text permalink
icon permalink
menu permalink
meter permalink
number-field permalink
overlay permalink
picker-button permalink
picker permalink
popover permalink
progress-bar permalink
radio permalink
search permalink
slider permalink
split-button permalink
swatch permalink
switch permalink
table permalink
tabs permalink
tags permalink
textfield permalink
toast permalink
tooltip permalink
top-nav permalink
Firefoxaccordion permalink
action-bar permalink
action-button permalink
action-group permalink
action-menu permalink
button-group permalink
button permalink
card permalink
checkbox permalink
dialog permalink
field-group permalink
field-label permalink
help-text permalink
icon permalink
menu permalink
meter permalink
number-field permalink
overlay permalink
picker-button permalink
picker permalink
popover permalink
progress-bar permalink
radio permalink
search permalink
slider permalink
split-button permalink
swatch permalink
switch permalink
table permalink
tabs permalink
tags permalink
textfield permalink
toast permalink
tooltip permalink
top-nav permalink
|
Summary of Changes Impacting FailuresChanges effecting horizontal alignmentCheckbox has been replaced by CloseButton and Label. Since ActionBar now has three child elements instead of two, the layout is no longer controlled with justify-content:space-between. Instead, the last element, ActionGroup, is aligned to the end of ActionBar with margin-inline-start: auto. Changes effecting vertical alignmentSince new tokens provide exact spacing from the top of ActionBar to the components within, align-content: center is no longer used as it would conflict with this exact spacing. Changes effecting shadow appearanceExpress theme drop shadow filter has increased in vertical sizing from 1px to 4px and increased in blur from 4px to 16px. Failure Pattern 1: Expected
Markup updates are needed to replace Checkbox with CloseButton and Label and to update Popover to reflect the new flex layout styles. A visual illustrating the new child components follows:
Failure Pattern 2: Expected
The failure which is noticeable on Express themes is a dramatic difference in drop shadow width and blur. Express drop shadow values have changed from 0 1 4 to 0 4 16, resulting in a much more pronounced shadow. |
|
The ActionBar changes have been merged in Spectrum CSS, and the release has been graduated. The full release is now: |
Westbrook
force-pushed
the
lunarfusion/action-bar-core-tokens
branch
from
4a10720 to
9d26083
Compare
Westbrook
reviewed
Nov 9, 2022
Comment on lines
203
to
215
| :host([emphasized]) .spectrum-ActionButton, | ||
| :host([emphasized]) .spectrum-CloseButton-UIIcon, | ||
| :host([emphasized]) ::slotted(sp-field-label) { | ||
| fill: var( | ||
| --mod-actionbar-emphasized-item-counter-color, | ||
| var(--spectrum-actionbar-emphasized-item-counter-color) | ||
| ); | ||
| color: var( | ||
| --mod-actionbar-emphasized-item-counter-color, | ||
| var(--spectrum-actionbar-emphasized-item-counter-color) | ||
| ); /* .spectrum-ActionBar--emphasized .spectrum-ActionButton, | ||
| * .spectrum-ActionBar--emphasized .spectrum-FieldLabel, | ||
| * .spectrum-ActionBar--emphasized .spectrum-CloseButton-UIIcon */ |
There was a problem hiding this comment.
@pfulton and @lunarfusion this would otherwise make these buttons "static white", right?
Does this point at the benefit of asking the design team to include a "static white" Field Label?
There was a problem hiding this comment.
@pfulton if this "static white"s the Buttons, should we look at having Action Group manage the variant of child buttons the way you added support for managing size that way?
Westbrook
reviewed
Nov 9, 2022
packages/action-bar/src/ActionBar.ts
Outdated
| @@ -71,6 +75,7 @@ export class ActionBar extends SpectrumElement { | |||
| public override render(): TemplateResult { | |||
| return html` | |||
| <sp-popover ?open=${this.open} id="popover"> | |||
| <sp-close-button class="close-button"></sp-close-button> | |||
There was a problem hiding this comment.
@pfulton it appears that the CSS implementation for this does not have an accessible label. Can we get clarify on what that should be "Close", "Deselect", etc.?
There was a problem hiding this comment.
Did we ever get content for this button, @pfulton?
There was a problem hiding this comment.
@Westbrook guidance from design and content strategy is to use the phrase "Clear selection". We'll make this change in the CSS docs in our next sprint to reflect this choice.
Westbrook
force-pushed
the
lunarfusion/action-bar-core-tokens
branch
5 times, most recently
from
b4b8537 to
57cc46d
Compare
Closed
1 task
13 tasks
blunteshwar
approved these changes
Jul 19, 2023
Westbrook
force-pushed
the
lunarfusion/action-bar-core-tokens
branch
from
062a3c8 to
24f52cf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Changes expected DOM structure from:
to
emphasizeddeliveryRelated issue(s)
Motivation and context
Core token migration
How has this been tested?
Types of changes
Checklist